Taxonomy paging and search for Categories and Tags. by kurzee · Pull Request #4749 · wordpress-mobile/WordPress-iOS

kurzee · 2016-01-28T22:24:52Z

This is required implementation for work on Menus management and also for the upcoming work on Categories and Tags management. There is more work to be done between categories and tags across the whole app, but will await a larger refactor with many more dependencies observed.

Refactors TaxonomyServiceRemote for paging and searching taxonomy results via REST and XML-RPC.
Refactors PostTagService for supporting search and paging via remotes.
This also resolves an issue seen in XML-RPC connections in which a category created via the app with a parent category was not setting the parent correctly according to the XML-RPC.

Note on PostCategoryService: I did not include the methods on PostCategoryService for supporting paging or search. Upon testing I noticed it will require additional refactoring that will have to pay careful attention to current dependencies of PostCategoryService and how it merges the PostCategory entities.

Testing should be done with both REST and XML-RPC sites.

Open "Settings" for a site.
The "Default Category" should be presented correctly.
Selecting the "Default Category" cell should present a site's categories.
Hitting the + button should create a new category for the site.
Creating a new category with a parent should work correctly, particularly on XML-RPC (broken previously)
Create a new post for a site.
When the post is posted, the default category should have been automatically assigned.
Create a post with a different category, the post should have the correct category.

Additional testing for the PostTagService will come with upcoming Menus PRs, since there are no current users of the PostTagService within the project.

Please review @aerych.

Category searching will require additional refactoring as the current merge method actually replaces all categories. Need to investigate dependencies before search can be supported via a merge that doesn’t delete entities.

aerych · 2016-01-29T15:31:21Z

Documentation comments need the /** opener vs /*

aerych · 2016-01-29T16:08:30Z

This looks identical to the REST version. Maybe its a good candidate to extract into a helper class vs duplication?

I'd like to keep these separate as the APIs are just a tad different and any further implementation would need to be treated as such.

aerych · 2016-01-29T16:55:07Z

Hiya @kurzee! Good stuff :) I left some nitpicky comments through through the code, all really minor.

I tested a wpcom blog and a Jetpack connected blog and the default category updated as expected, and was correctly reflected in post options. Very nice.

I tested a self-hosted-no-jetpack blog and confirmed the fix for parent/child categories.

The default category does not seem to be working for self-hosted blogs via XML-RPC. When adding a self-hosted blog, the default category in the app was "Uncategorized", ignoring what was set on the blog and selecting a default category in the app did not update the default category on the blog. From what I can tell, default_category, while a valid option, is not returned by wp.getOptions nor can it be set by calling wp.setOptions. At least this is the behavior when I try those calls. I've been looking through the XML-RPC API docs and I'm not finding any other way to get/set the default category. Maybe this is just not currently supported via XML-RPC? Should we hide the feature for those blogs?

…or nil responses.

kurzee · 2016-01-30T02:37:33Z

Thanks for the great review @aerych! I added new commits earlier and did some more testing.

I also spent some time trying to find a way to get/set that default category on XML-RPC and couldn't find a working method either... Strange that it isn't included in the XML-RPC.

I'd say we should keep the current default category implementation within the app as it allows the user to select a different default category for use within the app editor. This way, they don't have to keep selecting a category each time they open the editor.

aerych · 2016-02-01T15:26:31Z

This way, they don't have to keep selecting a category each time they open the editor.

I'm a little worried about either confusing a user, or breaking expectations when a change to the default category in the app is not reflected on the web (and vice versa). If we keep the default category selector for self-hosted folks, could we somehow clarify its usage?

kurzee · 2016-02-01T16:13:48Z

could we somehow clarify its usage?

I think that would be best so users understand that any change is only within the app. This is also the same for the "Default Post Format" option. I'll come up with some copy to place above the table section as a header label.

aerych · 2016-02-01T16:17:12Z

I'll come up with some copy to place above the table section as a header label.

Awesome. Thanks @kurzee !

kurzee · 2016-02-01T16:51:16Z

@aerych I'm going to create a separate issue/PR for the XML-RPC default category changes. There's a couple other changes that need to be made that are out of context with this PR. Let me know if you have any other notes on this PR.

aerych · 2016-02-01T19:07:44Z

Hiya @kurzee :)
I had one last very very nitpicky comment. But otherwise :)

kurzee · 2016-02-01T19:16:09Z

Thanks @aerych! Changed the variable name and added a helper function for logging those errors! Merging.

…g-and-search Taxonomy paging and search for Categories and Tags.

kurzee added 9 commits January 26, 2016 15:04

Refactor TaxonomyServiceRemotes to generalize/share methods.

99e9e32

Merging existing tags with newly fetched tags.

2dbe902

Reordering Taxonomy methods.

03feabf

Adds methods for remote taxonomy paging requests.

4420c15

Resolves issue with creating a new category with a parent, XMLRPC.

c2e8742

PostTag paging via PostTagService.

6938d80

Remote taxonomy searching.

3f252d6

PostTag and PostCategory service searching.

a530087

Removing previous additions of PostCategoryService searching.

9ce849a

Category searching will require additional refactoring as the current merge method actually replaces all categories. Need to investigate dependencies before search can be supported via a merge that doesn’t delete entities.

kurzee added Menus Tag/Category Management labels Jan 28, 2016

kurzee added this to the 6.0 milestone Jan 28, 2016

kurzee self-assigned this Jan 28, 2016

aerych reviewed Jan 29, 2016
View reviewed changes

kurzee added 5 commits January 29, 2016 11:39

Updates method for completion handlers after parameters.

6aebe1e

Comment corrections for documentation.

4bbf875

Adds types to responseObjects and prevents returning empty taxonomy f…

f560b7e

…or nil responses.

Using constants for taxonomy parameters.

d3602ae

Logging any errors for getting blog.

28e8e1d

kurzee mentioned this pull request Feb 1, 2016

Settings: Writing defaults for post category and format are only synced with admin users via REST #4756

Closed

Error handling function for nil blog or valid error.

46727d8

kurzee added a commit that referenced this pull request Feb 1, 2016

Merge pull request #4749 from wordpress-mobile/feature/taxonomy-pagin…

371e215

…g-and-search Taxonomy paging and search for Categories and Tags.

kurzee merged commit 371e215 into develop Feb 1, 2016

kurzee deleted the feature/taxonomy-paging-and-search branch February 1, 2016 19:17

Navigation Menu

Uh oh!

Taxonomy paging and search for Categories and Tags. - #4749

Taxonomy paging and search for Categories and Tags.#4749
kurzee merged 15 commits into
developfrom
feature/taxonomy-paging-and-search

kurzee commented Jan 28, 2016

Uh oh!

aerych Jan 29, 2016

Uh oh!

aerych Jan 29, 2016

Uh oh!

kurzee Jan 29, 2016

Uh oh!

aerych Jan 29, 2016

Uh oh!

aerych commented Jan 29, 2016

Uh oh!

kurzee commented Jan 30, 2016

Uh oh!

aerych commented Feb 1, 2016

Uh oh!

kurzee commented Feb 1, 2016

Uh oh!

aerych commented Feb 1, 2016

Uh oh!

kurzee commented Feb 1, 2016

Uh oh!

aerych commented Feb 1, 2016

Uh oh!

kurzee commented Feb 1, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

kurzee commented Jan 28, 2016

Uh oh!

aerych Jan 29, 2016

Choose a reason for hiding this comment

Uh oh!

aerych Jan 29, 2016

Choose a reason for hiding this comment

Uh oh!

kurzee Jan 29, 2016

Choose a reason for hiding this comment

Uh oh!

aerych Jan 29, 2016

Choose a reason for hiding this comment

Uh oh!

aerych commented Jan 29, 2016

Uh oh!

kurzee commented Jan 30, 2016

Uh oh!

aerych commented Feb 1, 2016

Uh oh!

kurzee commented Feb 1, 2016

Uh oh!

aerych commented Feb 1, 2016

Uh oh!

kurzee commented Feb 1, 2016

Uh oh!

aerych commented Feb 1, 2016

Uh oh!

kurzee commented Feb 1, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants